Skip to content

Fixed some memory leaks with http/3 #9336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
bryancall merged 1 commit into from
Feb 1, 2023
Merged

Fixed some memory leaks with http/3 #9336

bryancall merged 1 commit into from
Feb 1, 2023

Conversation

@bryancall
Copy link
Contributor

@bryancall bryancall commented Jan 27, 2023 •
edited
Loading

These numbers are with using freelists:

Before:
SUMMARY: AddressSanitizer: 9997094750 byte(s) leaked in 4587157 allocation(s).
After:
SUMMARY: AddressSanitizer: 137185838 byte(s) leaked in 77301 allocation(s).

@bryancall bryancall added this to the 10.0.0 milestone Jan 27, 2023
@bryancall bryancall requested a review from maskit January 27, 2023 19:29
@bryancall bryancall self-assigned this Jan 27, 2023
@bryancall
Copy link
Contributor Author

bryancall commented Jan 27, 2023

For reference this is where HQSession::remove_transaction is called:

Thread 12 "[ET_NET 9]" hit Breakpoint 1, Queue<HQTransaction, Continuation::Link_link>::remove (this=<optimized out>, e=<optimized out>) at /home/bcall/dev/apache/trafficserver/build-Linux_clang_h3_asan/../include/tscore/List.h:592
592       if (tail == e)
(gdb) bt
#0  Queue<HQTransaction, Continuation::Link_link>::remove (this=<optimized out>, e=<optimized out>)
    at /home/bcall/dev/apache/trafficserver/build-Linux_clang_h3_asan/../include/tscore/List.h:592
#1  HQSession::remove_transaction (this=0x620000010080, trans=0x626000012100) at ../../../proxy/http3/Http3Session.cc:57
#2  0x0000000000f59ebc in Http3App::_handle_bidi_stream_on_write_complete (this=<optimized out>, event=<optimized out>, vio=0x612000073170)
    at ../../../proxy/http3/Http3App.cc:354
#3  0x0000000000f57acd in Http3App::main_event_handler (this=0x62600000f100, event=103, data=<optimized out>) at ../../../proxy/http3/Http3App.cc:148
#4  0x0000000000fa9f5f in Continuation::handleEvent (this=<optimized out>, event=103, data=0x62c000004b40) at ../../../iocore/eventsystem/I_Continuation.h:227
#5  EThread::process_event (this=<optimized out>, e=0x62c000004b40, calling_code=<optimized out>) at ../../../iocore/eventsystem/UnixEThread.cc:152
#6  0x0000000000faae90 in EThread::process_queue (this=this@entry=0x7ffff0ad1800, NegativeQueue=NegativeQueue@entry=0x7fffe79400a0,
    ev_count=ev_count@entry=0x7fffe79400d0, nq_count=nq_count@entry=0x7fffe79400c0) at ../../../iocore/eventsystem/UnixEThread.cc:187
#7  0x0000000000fac572 in EThread::execute_regular (this=0x7ffff0ad1800) at ../../../iocore/eventsystem/UnixEThread.cc:265
#8  0x0000000000fadbcc in EThread::execute (this=0x7ffff0ad1800) at ../../../iocore/eventsystem/UnixEThread.cc:337
#9  0x0000000000fa78bc in spawn_thread_internal (a=0x606000040940) at ../../../iocore/eventsystem/Thread.cc:79
#10 0x00007ffff6cae12d in start_thread (arg=<optimized out>) at pthread_create.c:442
#11 0x00007ffff6d2fbc0 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

And Http3HeaderFramer::~Http3HeaderFramer:

Thread 12 "[ET_NET 9]" hit Breakpoint 2, Http3HeaderFramer::~Http3HeaderFramer (this=0x611000070fc0) at ../../../proxy/http3/Http3HeaderFramer.cc:40
40      {
(gdb) bt
#0  Http3HeaderFramer::~Http3HeaderFramer (this=0x611000070fc0) at ../../../proxy/http3/Http3HeaderFramer.cc:40
#1  0x0000000000f6329a in Http3Transaction::~Http3Transaction (this=0x626000012100) at ../../../proxy/http3/Http3Transaction.cc:324
#2  0x0000000000f635de in Http3Transaction::~Http3Transaction (this=0x611000070fc0) at ../../../proxy/http3/Http3Transaction.cc:323
#3  0x0000000000f59ebc in Http3App::_handle_bidi_stream_on_write_complete (this=<optimized out>, event=<optimized out>, vio=0x612000072870)
    at ../../../proxy/http3/Http3App.cc:354
#4  0x0000000000f57acd in Http3App::main_event_handler (this=0x62600000f100, event=103, data=<optimized out>) at ../../../proxy/http3/Http3App.cc:148
#5  0x0000000000fa9f5f in Continuation::handleEvent (this=<optimized out>, event=103, data=0x62c0000048a0) at ../../../iocore/eventsystem/I_Continuation.h:227
#6  EThread::process_event (this=<optimized out>, e=0x62c0000048a0, calling_code=<optimized out>) at ../../../iocore/eventsystem/UnixEThread.cc:152
#7  0x0000000000faae90 in EThread::process_queue (this=this@entry=0x7ffff0ad1800, NegativeQueue=NegativeQueue@entry=0x7fffe779c0a0,
    ev_count=ev_count@entry=0x7fffe779c0d0, nq_count=nq_count@entry=0x7fffe779c0c0) at ../../../iocore/eventsystem/UnixEThread.cc:187
#8  0x0000000000fac572 in EThread::execute_regular (this=0x7ffff0ad1800) at ../../../iocore/eventsystem/UnixEThread.cc:265
#9  0x0000000000fadbcc in EThread::execute (this=0x7ffff0ad1800) at ../../../iocore/eventsystem/UnixEThread.cc:337
#10 0x0000000000fa78bc in spawn_thread_internal (a=0x606000040940) at ../../../iocore/eventsystem/Thread.cc:79
#11 0x00007ffff6cae12d in start_thread (arg=<optimized out>) at pthread_create.c:442
#12 0x00007ffff6d2fbc0 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

maskit
maskit reviewed Jan 31, 2023
View reviewed changes
proxy/http3/Http3Session.cc
HQSession::remove_transaction(HQTransaction *trans)
{
this->_transaction_list.remove(trans);
delete trans;
Copy link
Member

@maskit maskit Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I want to delete transactions on the caller side, but I'll propose the change later.

Copy link
Member

@maskit maskit Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but I wonder why it leaks. Because all transactions in _transaction_list are deleted in the destructor of HQSession. See #9346

Copy link
Contributor Author

@bryancall bryancall Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HQSession::remove_transaction() is called it will remove the transaction from the _transaction_list and it will never be deleted.

Copy link
Member

@maskit maskit Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. Go ahead and merge this.

@bryancall bryancall mentioned this pull request Jan 31, 2023
Merged
@bryancall bryancall added the Leak label Jan 31, 2023
@bryancall bryancall merged commit a776a9f into apache:10-Dev Feb 1, 2023
bryancall added a commit to bryancall/trafficserver that referenced this pull request Feb 3, 2023
@bryancall
Fixed some memory leaks with http/3 (apache#9336)
682c7ba 
(cherry picked from commit a776a9f)
@bryancall bryancall modified the milestones: 10.0.0, Sometime Feb 3, 2023
bryancall added a commit that referenced this pull request Feb 3, 2023
@bryancall
Fixed some memory leaks with http/3 (#9336) (#9375)
271038d 
(cherry picked from commit a776a9f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maskit maskit maskit approved these changes

Assignees

@bryancall bryancall

Labels

HTTP/3 Leak

Projects

None yet

Milestone

Sometime

Development

Successfully merging this pull request may close these issues.

2 participants

@bryancall @maskit